Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove rowData and rowID props, use data and key #10523

Merged
merged 20 commits into from
Nov 17, 2021

Conversation

hanastasov
Copy link
Contributor

@hanastasov hanastasov commented Nov 16, 2021

Closes #10456

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@hanastasov hanastasov added ❌ status: awaiting-test PRs awaiting manual verification grid: general and removed ❌ status: awaiting-test PRs awaiting manual verification labels Nov 17, 2021
@zdrawku zdrawku self-requested a review November 17, 2021 08:06
@hanastasov hanastasov removed the ❌ status: awaiting-test PRs awaiting manual verification label Nov 17, 2021
Copy link
Member

@damyanpetev damyanpetev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no changes to packages in the package.json so no reason to update the lock file

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
<igx-icon *ngIf="trends.positive(cell.row.rowData)">trending_up</igx-icon>
<igx-icon *ngIf="trends.negative(cell.row.rowData)">trending_down</igx-icon>
<igx-icon *ngIf="trends.positive(cell.row.data)">trending_up</igx-icon>
<igx-icon *ngIf="trends.negative(cell.row.data)">trending_down</igx-icon>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, since this is in the markup and the cell template variable doesn't have type information (it's any) migrations probably won't pick this up and it looks unlikely for a custom migration right now.
In that case, we should definitely add an entry in the Upgrade Guide in the docs and include examples of templates the might need manual updating

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will definitely add an entry about that!

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@damyanpetev damyanpetev added the squash-merge Merge PR with "Squash and Merge" option label Nov 17, 2021
@kdinev kdinev merged commit b88b095 into master Nov 17, 2021
@kdinev kdinev deleted the hanastasov/removal-rowData-rowID-master branch November 17, 2021 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grid: general squash-merge Merge PR with "Squash and Merge" option version: 13.0.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add migration for rowData -> data
4 participants